<html>
<head><meta charset="utf-8"><title>Suppress drop terminators for fields in other variants · t-compiler · Zulip Chat Archive</title></head>
<h2>Stream: <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/index.html">t-compiler</a></h2>
<h3>Topic: <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html">Suppress drop terminators for fields in other variants</a></h3>

<hr>

<base href="https://rust-lang.zulipchat.com">

<head><link href="https://rust-lang.github.io/zulip_archive/style.css" rel="stylesheet"></head>

<a name="186151262"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186151262" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186151262">(Jan 21 2020 at 03:56)</a>:</h4>
<p>@eddyb I looked into your idea for making <code>const fn unwrap</code> work a bit tonight. It looks like we currently propagate dataflow state along the imaginary edge of a <code>FalseEdges</code> terminator. We would need to stop doing this, otherwise it will pollute the entry set of the <code>None</code> branch. For example, this code:</p>
<div class="codehilite"><pre><span></span><span class="k">fn</span> <span class="nf">unwrap</span><span class="o">&lt;</span><span class="n">T</span><span class="o">&gt;</span><span class="p">(</span><span class="n">opt</span>: <span class="nb">Option</span><span class="o">&lt;</span><span class="n">T</span><span class="o">&gt;</span><span class="p">)</span><span class="w"> </span>-&gt; <span class="nc">T</span><span class="w"> </span><span class="p">{</span><span class="w"></span>
<span class="w">    </span><span class="k">match</span><span class="w"> </span><span class="n">opt</span><span class="w"> </span><span class="p">{</span><span class="w"></span>
<span class="w">        </span><span class="nb">Some</span><span class="p">(</span><span class="n">inner</span><span class="p">)</span><span class="w"> </span><span class="o">=&gt;</span><span class="w"> </span><span class="n">inner</span><span class="p">,</span><span class="w"></span>
<span class="w">        </span><span class="nb">None</span><span class="w"> </span><span class="o">=&gt;</span><span class="w"> </span><span class="n">std</span>::<span class="n">process</span>::<span class="n">abort</span><span class="p">(),</span><span class="w"></span>
<span class="w">    </span><span class="p">}</span><span class="w"></span>
<span class="p">}</span><span class="w"></span>
</pre></div>


<p>results in the following CFG</p>
<p><a href="/user_uploads/4715/OcRsnrXxK9tn8M83w4REZDLR/false-edges.png" target="_blank" title="false-edges.png">false-edges.png</a> <a href="user_uploads/4715/iEfiSCIPld9ZKzucO6GkHrYJ/output.svg" target="_blank" title="user_uploads/4715/iEfiSCIPld9ZKzucO6GkHrYJ/output.svg">output.svg</a></p>
<div class="message_inline_image"><a href="/user_uploads/4715/OcRsnrXxK9tn8M83w4REZDLR/false-edges.png" target="_blank" title="false-edges.png"><img src="/user_uploads/4715/OcRsnrXxK9tn8M83w4REZDLR/false-edges.png"></a></div>



<a name="186151403"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186151403" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186151403">(Jan 21 2020 at 04:00)</a>:</h4>
<p>However, I assume the whole point of the <code>FalseEdges</code> terminator is to propagate dataflow state into other basic blocks, so maybe this is a no-go. BTW, <code>bb3</code> is the <code>None</code> branch.</p>



<a name="186151437"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186151437" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186151437">(Jan 21 2020 at 04:01)</a>:</h4>
<p><span class="user-mention" data-user-id="119009">@eddyb</span> Is there a way to work around this that I'm not seeing?</p>



<a name="186151674"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186151674" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186151674">(Jan 21 2020 at 04:08)</a>:</h4>
<p>Reading a bit, it seems like <code>FalseEdges</code> are needed becasue of pattern guards. Perhaps we can stop emitting them when no guard is present?</p>



<a name="186151725"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186151725" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186151725">(Jan 21 2020 at 04:08)</a>:</h4>
<p>Sorry about the blurriness, it looks like SVG isn't supported.</p>



<a name="186151740"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186151740" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186151740">(Jan 21 2020 at 04:09)</a>:</h4>
<p><a href="/user_uploads/4715/b-rGgfAP9k6JMmKBKYMguLZa/output.pdf" target="_blank" title="output.pdf">output.pdf</a></p>



<a name="186152104"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186152104" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186152104">(Jan 21 2020 at 04:19)</a>:</h4>
<p>I just noticed that, in <code>bb5</code>,<code>drop</code> is being called on <code>_3</code> right after it is moved into the return place. This probably means that I'm looking at MIR that's too early in the pipeline (this is the MIR that const qualification currently runs on).</p>



<a name="186152361"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186152361" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186152361">(Jan 21 2020 at 04:26)</a>:</h4>
<p>Okay, yeah I needed to be looking at the post-<code>ElaborateDrops</code> MIR.</p>



<a name="186152379"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186152379" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186152379">(Jan 21 2020 at 04:27)</a>:</h4>
<p><a href="user_uploads/4715/O_p7j119pdZNe7ItwAu_HEA9/output.pdf" target="_blank" title="user_uploads/4715/O_p7j119pdZNe7ItwAu_HEA9/output.pdf">output.pdf</a> </p>
<p><code>SimplifyCfg.elaborate_drops.after</code></p>



<a name="186153909"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186153909" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186153909">(Jan 21 2020 at 05:07)</a>:</h4>
<p>So now that I'm actually looking at the right stage, I see the problem I need to solve. We check the discriminant of <code>opt</code> (<code>_1</code>), then if it's <code>Some</code> we jump to <code>bb4</code>, which puts the value inside the option inside the return place. However, at the end of <code>bb4</code>, we check the discriminant <em>again</em> and <code>drop(_1)</code> if it has magically changed out from under us.</p>
<p>It seems we need to do some more dead code elimination after elaborate drops.</p>



<a name="186154186"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186154186" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186154186">(Jan 21 2020 at 05:14)</a>:</h4>
<p>One other thing I noticed is that we don't actually do <code>ElaborateDrops</code>on generic functions unless they are monomorphized. I think this means that we can't rely on <code>ElaborateDrops</code> to forbid reachable <code>Drop</code> terminators since we want pre-monomorphization errors.</p>



<a name="186154190"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186154190" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186154190">(Jan 21 2020 at 05:14)</a>:</h4>
<p>(btw, I'm sure a lot of the things I'm saying are obvious to you, I'm thinking out loud atm)</p>



<a name="186154837"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186154837" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186154837">(Jan 21 2020 at 05:34)</a>:</h4>
<p>So resetting a bit: It looks like we'll need to deal with <a href="/user_uploads/4715/b-rGgfAP9k6JMmKBKYMguLZa/output.pdf" target="_blank" title="output.pdf">this MIR</a>. We don't need to worry about the <code>drop</code> of <code>_3</code> in <code>bb5</code> because <code>check_consts</code> already looks for when a local is moved out of prior to being dropped. It's the drop of <code>_1</code> in <code>bb7</code> that is causing problems. I need to replicate whatever part of <code>elaborate_drops</code> is responsible for realizing that <code>_1</code> no longer needs to be dropped after we move out of it in <code>bb5[1]</code>.</p>



<a name="186154898"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186154898" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186154898">(Jan 21 2020 at 05:36)</a>:</h4>
<p><span class="user-mention silent" data-user-id="124288">oli</span> mentioned in <a href="https://github.com/rust-lang/rust/issues/66753" target="_blank" title="https://github.com/rust-lang/rust/issues/66753">#66753</a> that the <code>None</code> case was also causing problems. That's actually not true; I should have double-checked their comment at the time. The <code>drop</code> they refer to is in a cleanup block, so it is ignored by const-checking. The rationale being that unwinding during const-eval is basically an <code>abort</code> that triggers a compilation failure.</p>



<a name="186157840"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186157840" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> oli <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186157840">(Jan 21 2020 at 06:53)</a>:</h4>
<p>It's not in cleanup. It's reachable via bb0 -&gt; bb2 -&gt; bb3 -&gt; bb17 -&gt; bb7 -&gt; bb8 -&gt; bb16</p>



<a name="186157920"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186157920" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> oli <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186157920">(Jan 21 2020 at 06:54)</a>:</h4>
<p>basically after the <code>bar()</code> call there's a <code>match x { Some(_) =&gt; {}, None =&gt; drop(x) }</code> (the switch in bb8 does this)</p>



<a name="186163242"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186163242" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186163242">(Jan 21 2020 at 08:42)</a>:</h4>
<p>Ach you're right (for some reason I saw bb11 on your GH comment). However, it does look like there's no non-cleanup <code>drop</code> terminator on the <code>None</code> side for just <a href="/user_uploads/4715/b-rGgfAP9k6JMmKBKYMguLZa/output.pdf" target="_blank" title="output.pdf"><code>unwrap</code></a>.</p>
<div class="codehilite"><pre><span></span><span class="k">fn</span> <span class="nf">unwrap</span><span class="o">&lt;</span><span class="n">T</span><span class="o">&gt;</span><span class="p">(</span><span class="n">opt</span>: <span class="nb">Option</span><span class="o">&lt;</span><span class="n">T</span><span class="o">&gt;</span><span class="p">)</span><span class="w"> </span>-&gt; <span class="nc">T</span><span class="w"> </span><span class="p">{</span><span class="w"></span>
<span class="w">    </span><span class="k">match</span><span class="w"> </span><span class="n">opt</span><span class="w"> </span><span class="p">{</span><span class="w"></span>
<span class="w">        </span><span class="nb">Some</span><span class="p">(</span><span class="n">inner</span><span class="p">)</span><span class="w"> </span><span class="o">=&gt;</span><span class="w"> </span><span class="n">inner</span><span class="p">,</span><span class="w"></span>
<span class="w">        </span><span class="nb">None</span><span class="w"> </span><span class="o">=&gt;</span><span class="w"> </span><span class="n">std</span>::<span class="n">process</span>::<span class="n">abort</span><span class="p">(),</span><span class="w"></span>
<span class="w">    </span><span class="p">}</span><span class="w"></span>
<span class="p">}</span><span class="w"></span>
</pre></div>



<a name="186163569"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186163569" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186163569">(Jan 21 2020 at 08:49)</a>:</h4>
<p>So if the goal is to get exactly <code>unwrap</code> and <code>expect</code> working in const fns, I think we could do it by using the results of <code>maybe_inits</code>to detect when all fields of an ADT are uninitialized at a certain location.</p>



<a name="186163764"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186163764" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186163764">(Jan 21 2020 at 08:53)</a>:</h4>
<p>I'm not sure which of the categories you mentioned in your comment this belongs to. Maybe "independent solution"? I'm sure drop elaboration is already doing something similar. I also think we should talk about what our long-term goals are. How brittle a system are we comfortable given that someday people will be able to write <code>T: const Drop</code> (not actual syntax) to bypass this in the generic case? Is there every going to be a way to explain to the user why we cannot prove that a value isn't dropped?</p>



<a name="186163960"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186163960" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186163960">(Jan 21 2020 at 08:56)</a>:</h4>
<p>I will also look into the example you posted <span class="user-mention silent" data-user-id="124288">oli</span>, to see if I can apply <span class="user-mention silent" data-user-id="119009">eddyb</span>'s ideas about per-edge effects from <code>SwitchInt</code>s to remove some unnecessary <code>drop</code>s from the final MIR.</p>



<a name="186165803"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186165803" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186165803">(Jan 21 2020 at 09:25)</a>:</h4>
<p>I should go to bed. I'm rambling.</p>



<a name="186208416"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186208416" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186208416">(Jan 21 2020 at 17:51)</a>:</h4>
<p>at the very least we might be able to get simpler codegen (since I think today we rely on LLVM to remove the drop)</p>



<a name="186327101"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186327101" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186327101">(Jan 22 2020 at 20:24)</a>:</h4>
<p>Okay, I did not do a good job of identifying and explaining the various concerns here. I'll have another go:</p>



<a name="186327309"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186327309" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186327309">(Jan 22 2020 at 20:26)</a>:</h4>
<p>For <code>unwrap</code>, the only <code>Drop</code> terminator that's <em>not in a cleanup block</em> is along the <code>Some</code> edge. AFAICT, there's two shortcomings with drop elaboration currently. The first, which <span class="user-mention" data-user-id="119009">@eddyb</span>  pointed out, is that <code>MaybeInitializedPlaces</code>and drop elaboration doesn't incorporate the information we get when we switch on a discriminant. As a result, oli's example, which is slightly more complex than <code>unwrap</code>, <em>does</em> have a non-cleanup <code>Drop</code> along the <code>None</code> path. I mistakenly said that this was not the case (sorry oli!).</p>



<a name="186327674"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186327674" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186327674">(Jan 22 2020 at 20:30)</a>:</h4>
<p>The second shortcoming is that, when we move out of all fields of an ADT without custom <code>Drop</code> glue, we still emit a <code>Drop</code> terminator for that ADT. This occurs along the <code>Some</code> branch of <code>unwrap</code>. I posted some erroneous MIR diagrams for <code>unwrap</code> that didn't show this, which were generated from an experimental branch into which I had introduced a bug. Here's an accurate diagram of the MIR for unwrap <strong>after drop elaboration</strong> (and cleanup)</p>
<p><a href="/user_uploads/4715/1wCN8r1Y1b5WPQuIf4BNZy7a/pasted_image.png" target="_blank" title="pasted_image.png">pasted image</a></p>
<div class="message_inline_image"><a href="/user_uploads/4715/1wCN8r1Y1b5WPQuIf4BNZy7a/pasted_image.png" target="_blank" title="pasted image"><img src="/user_uploads/4715/1wCN8r1Y1b5WPQuIf4BNZy7a/pasted_image.png"></a></div>



<a name="186328053"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186328053" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186328053">(Jan 22 2020 at 20:34)</a>:</h4>
<p>And here's the MIR <strong>before drop elaboration</strong>.</p>
<p><a href="user_uploads/4715/BmtN4jnQSU67gukd-sxhbzJf/pasted_image.png" target="_blank" title="user_uploads/4715/BmtN4jnQSU67gukd-sxhbzJf/pasted_image.png">pasted image</a></p>
<div class="message_inline_image"><a href="user_uploads/4715/BmtN4jnQSU67gukd-sxhbzJf/pasted_image.png" target="_blank" title="pasted image"><img src="user_uploads/4715/BmtN4jnQSU67gukd-sxhbzJf/pasted_image.png"></a></div>



<a name="186328247"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186328247" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186328247">(Jan 22 2020 at 20:36)</a>:</h4>
<p>We end up checking the discriminant of <code>_1</code> twice because the <code>drop(_1)</code> that is originally in basic block 6 is classified as an "open" drop by drop elaboration. This is because some fields of <code>_1</code> have been moved out of.</p>



<a name="186328502"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186328502" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186328502">(Jan 22 2020 at 20:39)</a>:</h4>
<p>I want to stop classifying drops of locals without custom drop glue as "open" if we can prove that all of their fields have been moved out of. I haven't fully considered the ramifications of this, but it seems tractable. Another approach would be const-propagation for calls to <code>discriminant</code> along with DCE, but this would have to occur very late in the pipeline and would not be accessible during const-checking.</p>



<a name="186331526"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186331526" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186331526">(Jan 22 2020 at 21:12)</a>:</h4>
<p>To detect when all fields have been moved out of the argument to a <code>Drop</code> terminator, I would need to:</p>
<ul>
<li>iterate over all fields in the <code>AdtDef</code> for the local</li>
<li>see if we have a move path for each field. If one exists, see if it is maybe init. If one does not exist, that field has the same drop flag state as the local and we are done.</li>
<li>If all fields are definitely uninit (!maybe_init), we can remove the <code>Drop</code> terminator entirely. Otherwise, recurse into the <code>AdtDef</code> for the <code>maybe_init</code> field, to see if <em>it</em> has had all its fields moved out of.</li>
</ul>



<a name="186331889"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186331889" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186331889">(Jan 22 2020 at 21:17)</a>:</h4>
<p>For <code>enum</code>s, we would also need to fix the first shortcoming (marking fields that are not in the live variant as uninit) for this to work generally. <code>Option</code> is a bit of a special case, as it has exactly one field across all of its variants, so it's not dependent on this.</p>



<a name="186332145"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186332145" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186332145">(Jan 22 2020 at 21:20)</a>:</h4>
<p>There's a bit of extra complexity because three move paths are involved when moving out of <code>Some</code>: <code>_1</code>, <code>_1 as Some</code>, and <code>(_1 as Some).0</code>. Only the last one is marked as uninitialized.</p>



<a name="186433599"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186433599" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186433599">(Jan 23 2020 at 20:48)</a>:</h4>
<p><a href="https://github.com/rust-lang/rust/issues/68493" target="_blank" title="https://github.com/rust-lang/rust/issues/68493">#68493</a> should cause drop elaboration to stop emitting spurious <code>Drop</code> terminators when a value is moved from <code>Option::Some(_)</code>. I'll start working on <span class="user-mention silent" data-user-id="119009">eddyb</span>'s idea next, which would result in a similar improvement for enums that have more than one field across all variants (e.g., <code>Result</code>).</p>



<a name="186442130"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186442130" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186442130">(Jan 23 2020 at 22:13)</a>:</h4>
<p>Hmm, my one-line change results in incorrect codegen for <code>unwrap_or</code>. It doesn't clear the drop flag for <code>(_1 as Some).0</code> when taking the <code>None</code> branch. I'll need to do some more research about how this is supposed to work.</p>



<a name="186444075"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186444075" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> centril <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186444075">(Jan 23 2020 at 22:38)</a>:</h4>
<p><span class="user-mention" data-user-id="118594">@ecstatic-morse</span> does that PR affect static analysis as well?</p>



<a name="186444130"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186444130" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthew Jasper <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186444130">(Jan 23 2020 at 22:39)</a>:</h4>
<p>It doesn't</p>



<a name="186444337"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186444337" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> centril <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186444337">(Jan 23 2020 at 22:41)</a>:</h4>
<p>ah, good; then I think I followed things right</p>



<a name="186444475"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186444475" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186444475">(Jan 23 2020 at 22:43)</a>:</h4>
<p><span class="user-mention" data-user-id="116118">@Matthew Jasper</span> do you happen to know where I could find info on the high-level design of drop elaboration? Normally I use <code>git blame</code> and look at the PR summary, but the commit for most of <code>elaborate_drops</code> was the one that moved it into <code>librustc_mir</code>.</p>



<a name="186444577"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186444577" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186444577">(Jan 23 2020 at 22:44)</a>:</h4>
<p>I will be able to fumble through just from reading the code, but I'm pretty slow.</p>



<a name="186444619"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186444619" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthew Jasper <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186444619">(Jan 23 2020 at 22:45)</a>:</h4>
<p>Elaborate drops is old and undocumented. Comment in the source are probably all that exists. Maybe (but probably not) there's something in the Rustc guide.</p>



<a name="186444670"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186444670" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186444670">(Jan 23 2020 at 22:45)</a>:</h4>
<p>Yeah, I checked the guide as well and didn't find anything <span aria-label="confused" class="emoji emoji-1f615" role="img" title="confused">:confused:</span></p>



<a name="186444738"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186444738" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> centril <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186444738">(Jan 23 2020 at 22:46)</a>:</h4>
<p>I was wondering whether <a href="https://github.com/rust-lang/reference/pull/514" target="_blank" title="https://github.com/rust-lang/reference/pull/514">https://github.com/rust-lang/reference/pull/514</a> would be useful, but that's probably unrelated?</p>



<a name="186444767"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186444767" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthew Jasper <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186444767">(Jan 23 2020 at 22:46)</a>:</h4>
<p>It's unrelated, unfortunately.</p>



<a name="186444780"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186444780" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> centril <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186444780">(Jan 23 2020 at 22:47)</a>:</h4>
<p>(Also that lovely PR has sat a around for a long time unmerged, but that's also unrelated, but cc <span class="user-mention" data-user-id="116083">@pnkfelix</span>)</p>



<a name="186445186"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186445186" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186445186">(Jan 23 2020 at 22:53)</a>:</h4>
<p><a href="https://github.com/rust-lang/rust/issues/33622" target="_blank" title="https://github.com/rust-lang/rust/issues/33622">#33622</a> (found via transitive git blame) is the genesis of drop elaboration. I wonder if <span class="user-mention" data-user-id="126804">@Ariel Ben-Yehuda</span> has any thoughts on how to be more efficient around enums? This was implemented in 2016 and I don't remember what I had for lunch on Monday, so I would guess no XD.</p>



<a name="186678045"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186678045" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186678045">(Jan 27 2020 at 12:46)</a>:</h4>
<p><span class="user-mention" data-user-id="118594">@ecstatic-morse</span> I don't understand how there can only be a <code>Drop</code> on the <code>Some</code> branch</p>



<a name="186678062"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186678062" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186678062">(Jan 27 2020 at 12:46)</a>:</h4>
<p>oh, <em>unwrap</em></p>



<a name="186678150"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186678150" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186678150">(Jan 27 2020 at 12:48)</a>:</h4>
<p><span class="user-mention" data-user-id="118594">@ecstatic-morse</span> I understand my confusion now, at some point I stopped thinking of <code>Option::unwrap</code> and more of a <code>match</code> in general (you're right about <code>unwrap_or</code>, it's a better example of what I was imagining)</p>



<a name="186864472"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186864472" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186864472">(Jan 29 2020 at 08:19)</a>:</h4>
<p>Sorry <span class="user-mention" data-user-id="119009">@eddyb</span>, I had some distractions the past few days. I think I now have the proper modifications to drop elaboration in <a href="https://github.com/rust-lang/rust/issues/68528" target="_blank" title="https://github.com/rust-lang/rust/issues/68528">#68528</a> to be more efficient around fieldless enum variants. You can see the effects on the MIR for <code>Option::unwrap_or</code> below.</p>
<p><a href="/user_uploads/4715/lqzMPnW4njWGBe93KG0BRBcm/pasted_image.png" target="_blank" title="pasted_image.png">Before</a></p>
<div class="message_inline_image"><a href="/user_uploads/4715/lqzMPnW4njWGBe93KG0BRBcm/pasted_image.png" target="_blank" title="Before"><img src="/user_uploads/4715/lqzMPnW4njWGBe93KG0BRBcm/pasted_image.png"></a></div><p><a href="/user_uploads/4715/KAV6uA8a_Myu02n2spCrWOca/pasted_image.png" target="_blank" title="pasted_image.png">After</a></p>
<div class="message_inline_image"><a href="/user_uploads/4715/KAV6uA8a_Myu02n2spCrWOca/pasted_image.png" target="_blank" title="After"><img src="/user_uploads/4715/KAV6uA8a_Myu02n2spCrWOca/pasted_image.png"></a></div><p>Even without <em>any</em> changes to drop elaboration, implementing your idea is a pretty significant <a href="https://perf.rust-lang.org/compare.html?start=8ad83afe5bcfa983a24b6f720c9ef389350f414b&amp;end=3828cd570d1515093538246582353d44c84220db" target="_blank" title="https://perf.rust-lang.org/compare.html?start=8ad83afe5bcfa983a24b6f720c9ef389350f414b&amp;end=3828cd570d1515093538246582353d44c84220db">performance win</a>. Assuming I've not introduced any miscompilations, if the top-line numbers for <code>syn-opt</code> are anywhere close to reality, I would be overjoyed.</p>



<a name="186898286"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186898286" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186898286">(Jan 29 2020 at 15:52)</a>:</h4>
<p>-9% on syn-opt?!</p>



<a name="186898290"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186898290" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186898290">(Jan 29 2020 at 15:52)</a>:</h4>
<p>this feels too good to be true</p>



<a name="186909813"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186909813" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186909813">(Jan 29 2020 at 17:44)</a>:</h4>
<p>Agreed. I'm not sure how to verify that I've not introduced a miscompilation. that PR also includes my changes to data flow which is a small perf gain on its own. I expected to see a small regression in check builds and a small boost in codegened builds, but 10% is beyond the pale.</p>



<a name="186915819"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186915819" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186915819">(Jan 29 2020 at 18:48)</a>:</h4>
<p>I added a proper summary to <a href="https://github.com/rust-lang/rust/issues/68528" target="_blank" title="https://github.com/rust-lang/rust/issues/68528">#68528</a></p>



<a name="186923506"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186923506" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Zoxc <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186923506">(Jan 29 2020 at 20:05)</a>:</h4>
<p>There are two large regressions though, <a href="https://perf.rust-lang.org/compare.html?start=d55f3e9f1da631c636b54a7c22c1caccbe4bf0db&amp;end=0077a7aa11ebc2462851676f9f464d5221b17d6a&amp;stat=wall-time" target="_blank" title="https://perf.rust-lang.org/compare.html?start=d55f3e9f1da631c636b54a7c22c1caccbe4bf0db&amp;end=0077a7aa11ebc2462851676f9f464d5221b17d6a&amp;stat=wall-time">https://perf.rust-lang.org/compare.html?start=d55f3e9f1da631c636b54a7c22c1caccbe4bf0db&amp;end=0077a7aa11ebc2462851676f9f464d5221b17d6a&amp;stat=wall-time</a></p>



<a name="186924254"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186924254" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186924254">(Jan 29 2020 at 20:13)</a>:</h4>
<p><span class="user-mention" data-user-id="116466">@Zoxc</span> I suspect there is a lot of variance for wall-time on clean incremental builds since they finish so quickly, although the "noise run" link is broken at the top of that page.</p>



<a name="186926317"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186926317" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186926317">(Jan 29 2020 at 20:38)</a>:</h4>
<p>cc <span class="user-mention" data-user-id="116122">@simulacrum</span> ^</p>



<a name="186926411"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186926411" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186926411">(Jan 29 2020 at 20:39)</a>:</h4>
<p>Compare to <a href="https://perf.rust-lang.org/compare.html?start=a237641c7df8125b89b8f9c2a3594964ba8188f8&amp;end=c3681d62ee4bb849e87a2ec5d663afc34ac05d85&amp;stat=wall-time" target="_blank" title="https://perf.rust-lang.org/compare.html?start=a237641c7df8125b89b8f9c2a3594964ba8188f8&amp;end=c3681d62ee4bb849e87a2ec5d663afc34ac05d85&amp;stat=wall-time">https://perf.rust-lang.org/compare.html?start=a237641c7df8125b89b8f9c2a3594964ba8188f8&amp;end=c3681d62ee4bb849e87a2ec5d663afc34ac05d85&amp;stat=wall-time</a>, but yes, I would expect high variance</p>



<a name="186926427"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186926427" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186926427">(Jan 29 2020 at 20:40)</a>:</h4>
<p>(that's a "noop" build)</p>



<a name="186926578"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186926578" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> lqd <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186926578">(Jan 29 2020 at 20:41)</a>:</h4>
<p>this new perf run with a +0.1% and -0.7% instructions resulting in +800ms seems high variance, and those numbers were actually reversed on the previous perf run</p>



<a name="186926645"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186926645" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> lqd <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186926645">(Jan 29 2020 at 20:42)</a>:</h4>
<p>the "noop build" has a 47% regression in wall-time on cranelift ? :)</p>



<a name="186926903"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186926903" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186926903">(Jan 29 2020 at 20:44)</a>:</h4>
<p>It appears that the no-op build for <code>syn-opt</code> has some variance in <code>I-count</code> measurements as well. I suspect that my results for <code>syn-opt</code> are the combination of a legitimate perf gain with some helpful variance (hence the question mark). Those savings aren't reflected in wall time.</p>



<a name="186927172"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186927172" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Dylan MacKenzie (ecstatic-morse) <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186927172">(Jan 29 2020 at 20:48)</a>:</h4>
<p>In any case, I'm quite happy with 2-4% improvements in wall time for a lot of real-world crates. I wonder what's up with <code>tokio-webpush-simple-opt</code> though. Maybe I've done something that's unsound in the presence of generators?</p>



<a name="186927366"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186927366" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Zoxc <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186927366">(Jan 29 2020 at 20:50)</a>:</h4>
<p><span class="user-mention" data-user-id="116122">@simulacrum</span> That's a lot of variance on that benchmark</p>



<a name="186927421"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186927421" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186927421">(Jan 29 2020 at 20:51)</a>:</h4>
<p>Yes? I mean, what do you expect?</p>



<a name="186927436"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186927436" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186927436">(Jan 29 2020 at 20:51)</a>:</h4>
<p>this is measuring wall time from probably one or two runs</p>



<a name="186927489"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186927489" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186927489">(Jan 29 2020 at 20:52)</a>:</h4>
<p>for something that takes ~2 seconds, and is highly dependent on disk performance I imagine</p>



<a name="186949732"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Suppress%20drop%20terminators%20for%20fields%20in%20other%20variants/near/186949732" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> nagisa <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Suppress.20drop.20terminators.20for.20fields.20in.20other.20variants.html#186949732">(Jan 30 2020 at 02:21)</a>:</h4>
<p>That new mir graph looks glorious.</p>



<hr><p>Last updated: Aug 07 2021 at 22:04 UTC</p>
</html>